Compile variable features #635

simoncozens · 2022-07-19T12:44:13Z

This is a work in progress. The idea is this: when we are compiling a variable font from designspace, most of the time the feature files are exactly the same; the only thing which differs is the "implicit" rules created from anchors and kerning. Currently we build each font independently, and this means we compile binary features N times (where N=number of masters), and then merge them at the end. When there are overflows, compiling binary features N times is slow. Instead, this PR holds off the compilation of GSUB/GPOS tables to the end, and uses "variable" feature writers which describe how the kerning and anchors vary across the designspace. i.e. instead of:

### MutatorMathTest-LightCondensed ###
lookup kern_ltr {
    lookupflag IgnoreMarks;
    pos A J -10;
} kern_ltr;

### MutatorMathTest-BoldCondensed ###
lookup kern_ltr {
    lookupflag IgnoreMarks;
    pos A J -20;
} kern_ltr;

and compiles the features twice, it produces

lookup kern_ltr {
    lookupflag IgnoreMarks;
    pos A J (wght=0:-10 wght=1000:-20);
} kern_ltr;

and compiles the features once.

It's a lot faster.

Curs feature writer is untested.
There's some problem with enumerated kerns.
I'm not sure I trust it yet.

Lib/ufo2ft/__init__.py

madig · 2022-08-02T18:51:12Z

So to trigger this, a font project must make sure to have the same exact feature file text in every source, meaning that you have to use variable feature syntax if there are differences in some parts of the Designspace?

madig · 2022-08-02T18:58:14Z

Lib/ufo2ft/featureCompiler.py

@@ -92,6 +95,10 @@ def __init__(self, ufo, ttFont=None, glyphSet=None, **kwargs):

        glyphOrder = ttFont.getGlyphOrder()
        if glyphSet is not None:
+            if set(glyphOrder) != set(glyphSet.keys()):
+                print("Glyph order incompatible")


This should probably be a logger.error. Also, the glyph set is being tested, not the order.

this is just before an assert so it doesn't matter much if we print or do logger.error, an AssertionError should never trigger and is not something client can expect to recover from. It's just the error message for the hard crash, it could be moved to the assert optional string parameter, if you want.

simoncozens · 2022-08-02T19:03:28Z

Yes, it doesn’t attempt to do a text/AST-based merge of the feature files. At least not at this stage - maybe that can come later. I’m fairly sure that this will cover a good number of font projects, and those that it doesn’t get cover will get the old style merge.

madig · 2022-08-03T13:22:07Z

Lib/ufo2ft/featureWriters/baseFeatureWriter.py

        if compiler is not None:
            # The result is cached in the compiler instance, so if another
            # writer requests one it is not compiled again.
            if hasattr(compiler, "_gsub"):
                return compiler._gsub

            glyphOrder = compiler.ttFont.getGlyphOrder()
+            fvar = compiler.ttFont.get("fvar")
+            if fvar:
+                feafile = copy.deepcopy(feafile)


Why the deepcopy?

indeed, why?

Lib/ufo2ft/featureWriters/variableRulesWriter.py

madig · 2022-08-03T14:26:11Z

In one of our fonts, this seems to drop some kerning pairs but not others (can't check if this is just the kern splitting because the font doesn't compile without this PR) 🤔 weird, drilling some more.

Also, the Rules writer is not hooked up and justr dropping it into the default feature writer list doesn't seem to do anything.

madig · 2022-08-03T14:38:33Z

Lib/ufo2ft/featureWriters/variableRulesWriter.py

+                    self._conditionsets.append(conditionset)
+                else:
+                    cs_name = "ConditionSet%i" % self._conditionsets.index(conditionset)
+                # XXX


madig · 2022-08-03T14:58:17Z

So, I get rules to work when I graft the GSUB <FeatureVariations> from a previous ufo2ft and adapt the lookup numbers. This means that the rules writer should do whatever varLib is doing I guess.

Notes:

The writer outputs values in design space. There seems to be no official spec for this so I'll need to check feaLib
The writer outputs integer locations, where Designspaces can have floating point ones.

simoncozens · 2022-08-03T15:22:13Z

I disabled the rules writer because varLib also handles designspace rules during the merge and I was ending up with them being emitted twice.

madig · 2022-08-03T16:12:04Z

Weird. For my test font, no GSUB variations are emitted without the rules writer.

madig · 2022-08-03T16:55:37Z

Now we do approximately what varLib is doing with rules, though compared to a reference, the numbers and lookups are off in some places. Maybe I use overlayFeatureVariations wrong. At least with inspection by eye, stuff seems to work as expected.

There still remains a difference in kern behavior I need to investigate.

madig · 2022-08-03T19:09:54Z

Also todo: The test should include disjoint kerning pairs for different scripts, to ensure we handle disjoint kerning in different masters correctly (i.e. Armenian only in one, Greek only in the other).

madig · 2022-08-08T14:10:29Z

I disabled the rules writer because varLib also handles designspace rules during the merge and I was ending up with them being emitted twice.

Oh, I know what's going on. My test sources happen to trigger the compile-features-once-but-later code path, so varLib adds feature variations and then we overwrite them when running the feature writers. This also seems to affect kerning.

madig · 2022-08-08T16:59:32Z

Next, my test source has e.g. an r vs d class-based kern that gets lost when compiling features later for VFs. 🤔

simoncozens · 2022-08-17T06:26:55Z

You're using non-public test sources, which makes it difficult for other people to repro... I'll see what I can find.

simoncozens · 2022-08-17T07:15:12Z

Note to self: _featuresCompatible will return false if there are any ligature caret positions, which glyphsLib writes into the feature file (with per-master-file variations). Need to think how to handle these.

madig · 2022-08-17T09:44:00Z

Woops, yeah. I'll be back at work next week and can see if I can make a reproducer. For now, maybe just compile the font with identical features with 2.28.0 and then with this branch and compare kerning.

I thought I turned ligature writing off? Should be in https://github.com/googlefonts/glyphsLib/releases/tag/v6.0.7

anthrotype · 2022-08-17T14:26:44Z

yeah, the GdefFeatureWriter in ufo2ft should handle both the GlyphClassDefs (using public.openTypeCategories in the ufo lib) and the LigatureCarets (from the glyph anchors named "caret_", "vcaret_")

madig · 2022-08-23T11:38:08Z

So, I tried to come up with a minimal reproducer, but so far, no luck. What did seem to work was to make a union of all kerning values over all sources, putting in missing ones as zero-kerns. The specific pair I was looking at was actually "missing" such a zero-kern in one corner of the design space. Maybe that's what's missing?

madig · 2022-08-23T13:44:35Z

varLib allows "sparse kerning", i.e. if Light and bold have a value but Regular doesn't, the value at Regular is interpolated rather than set to zero. So, filling in zero for missing kern pairs seems like the wrong thing to do.

madig · 2022-08-23T15:54:59Z

In my testing with a proprietary font, it seems the kerning is there, but it is not used, as if the shaper was stopping earlier. Subsetting the font to contain just the two glyphs I want to look at makes the kern work as expected. Will try to reproduce with an OSS font.

madig · 2022-08-23T16:08:23Z

Found an open font that repros: Roboto Serif with "py".

simoncozens · 2022-08-24T15:18:34Z

Here is a very minimal reproducer:
RobotoSerif-Mini.glyphs.gz

madig · 2022-08-24T16:00:22Z

(The last commit should be using sets instead of lists for members in side(1|2)Groups but I don't want to anger the determinism gods)

simoncozens · 2022-08-25T14:31:40Z

With your patch and fonttools/fonttools#2776, I think this is good now.

It has a slight change of behaviour compared to the past in respect of "empty" kern pairs, but I think this is for the better. If an intermediate master previously did not define a kern value for a pair which had kerning in other masters, we would assume that value was zero. For example, if you had Regular=-100, Bold=-200 and no kern defined in the SemiBold, you would get a kerning value of zero. Presumably designers would have realised that they didn't mean that, and put an explicit kern in there.

~~But now we can do better than that: if there is no explicit kern for an intermediate master, it will be interpolated. This should make people's kerning processes a lot faster...~~

simoncozens · 2023-12-15T12:46:59Z

Ship it! :-)

* Using this version of otRound allows us to reuse a lot more code * A couple of variable scalar / designspace utility functions * Allow compileGSUB to know about fvar tables

This is the same as a FeatureCompiler but it knows its designspace and it also loads the RulesFeatureWriter if there are any designspace rules

Add test for different feature writers Fix up test expectations This should now test the variable features

…ling variable features

in googlefonts/ufo2ft#635 that symbol is no longer exported, better to import from its actual location

anthrotype · 2024-01-11T11:29:40Z

Lib/ufo2ft/featureWriters/markFeatureWriter.py

 from ufo2ft.constants import INDIC_SCRIPTS, USE_SCRIPTS
 from ufo2ft.featureWriters import BaseFeatureWriter, ast
 from ufo2ft.util import (
    classifyGlyphs,
-    quantize,


@simoncozens I had to change glyphsLib's markFeatureWriter to import quantize from ufo2ft.util otherwise we get ImportError with this PR:
googlefonts/glyphsLib@ae08b26

oh I had forgotten that you already have a PR for that, sorry: googlefonts/glyphsLib#957

ok so we shall merge that one once we release this

this can be useful to debugging, comparing build time of new vs old build pipeline, or if one wants to ensure no binary changes occur when updating the compiler

anthrotype · 2024-01-11T12:36:14Z

@simoncozens hope you don't mind if I added a variableFeatures=True compile option that we can use from fontmake to force disable this (52ca46a), just in case

See googlefonts/fontmake#1064

…OTL tables this is meant to work with googlefonts/ufo2ft#635

simoncozens force-pushed the main branch from 6c05d0c to 72b2459 Compare July 20, 2022 15:20

simoncozens force-pushed the compile-variable-features branch from 183d19a to 737fffc Compare July 22, 2022 13:10

simoncozens mentioned this pull request Jul 27, 2022

Variable anchor positioning in sparse masters #640

Closed

simoncozens force-pushed the compile-variable-features branch from b794e7d to 458ef3b Compare August 1, 2022 09:57

madig reviewed Aug 2, 2022

View reviewed changes

Lib/ufo2ft/__init__.py Outdated Show resolved Hide resolved

madig reviewed Aug 2, 2022

View reviewed changes

madig reviewed Aug 3, 2022

View reviewed changes

Lib/ufo2ft/featureWriters/variableRulesWriter.py Outdated Show resolved Hide resolved

madig reviewed Aug 3, 2022

View reviewed changes

simoncozens added 12 commits January 10, 2024 10:15

Util scaffolding

0fae115

* Using this version of otRound allows us to reuse a lot more code * A couple of variable scalar / designspace utility functions * Allow compileGSUB to know about fvar tables

Here come the variable feature writer implementations

dadd828

Add a VariableFeatureCompiler

f1b3ef4

This is the same as a FeatureCompiler but it knows its designspace and it also loads the RulesFeatureWriter if there are any designspace rules

Hook up variable feature compilation to __init__.py

dd278fc

Some tests for variable feature writing

d0252b6

Add test for different feature writers Fix up test expectations This should now test the variable features

Slightly improve the subclassing

f997b24

Stash and return glyphsets, then find the default glyphset when compi…

5122946

…ling variable features

On reflection these not needed

6473bbb

Use shiny new fonttools API

a591759

otRound the position when building a variable anchor

30b6be6

Use getAnchor to get variable ligcarets

809324f

Log progress

1f3f179

anthrotype force-pushed the compile-variable-features branch from 7d06369 to 1f3f179 Compare January 10, 2024 10:15

remove unused parameter

91e76cd

anthrotype force-pushed the compile-variable-features branch from 822c4b4 to 91e76cd Compare January 10, 2024 11:50

anthrotype added a commit to googlefonts/glyphsLib that referenced this pull request Jan 11, 2024

markFeatureWriter: import quantize from ufo2ft.util

ae08b26

in googlefonts/ufo2ft#635 that symbol is no longer exported, better to import from its actual location

anthrotype reviewed Jan 11, 2024

View reviewed changes

anthrotype added 2 commits January 11, 2024 12:21

add compile option to force variableFeatures=False

52ca46a

this can be useful to debugging, comparing build time of new vs old build pipeline, or if one wants to ensure no binary changes occur when updating the compiler

be more generic in log message emitted for either TTF or CFF2 VFs

207f247

anthrotype added a commit to googlefonts/fontmake that referenced this pull request Jan 11, 2024

add option to disable ufo2ft variable FEA, restore varLib merging of …

72c30cf

…OTL tables this is meant to work with googlefonts/ufo2ft#635

anthrotype mentioned this pull request Jan 11, 2024

add option to disable ufo2ft variable FEA, restore varLib merging of OTL tables googlefonts/fontmake#1064

Merged

anthrotype added 2 commits January 11, 2024 13:35

remove unused parameter

a375806

restore log message that got lost in #801 refactoring

a782bbf

anthrotype merged commit 657cb64 into main Jan 11, 2024
9 checks passed

anthrotype deleted the compile-variable-features branch January 11, 2024 15:13

anthrotype restored the compile-variable-features branch January 11, 2024 15:13

khaledhosny deleted the compile-variable-features branch January 13, 2024 18:37

anthrotype added a commit to googlefonts/fontmake that referenced this pull request Jan 18, 2024

add option to disable ufo2ft variable FEA, restore varLib merging of …

6ec4d81

…OTL tables this is meant to work with googlefonts/ufo2ft#635

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Compile variable features #635

Compile variable features #635

simoncozens commented Jul 19, 2022 •

edited

Loading

madig commented Aug 2, 2022

madig Aug 2, 2022 •

edited

Loading

anthrotype Nov 28, 2023

simoncozens commented Aug 2, 2022

madig Aug 3, 2022

anthrotype Dec 5, 2023

madig commented Aug 3, 2022 •

edited

Loading

madig Aug 3, 2022

madig commented Aug 3, 2022 •

edited

Loading

simoncozens commented Aug 3, 2022

madig commented Aug 3, 2022

madig commented Aug 3, 2022

madig commented Aug 3, 2022

madig commented Aug 8, 2022

madig commented Aug 8, 2022

simoncozens commented Aug 17, 2022

simoncozens commented Aug 17, 2022

madig commented Aug 17, 2022

anthrotype commented Aug 17, 2022

madig commented Aug 23, 2022 •

edited

Loading

madig commented Aug 23, 2022

madig commented Aug 23, 2022

madig commented Aug 23, 2022

simoncozens commented Aug 24, 2022

madig commented Aug 24, 2022

simoncozens commented Aug 25, 2022 •

edited

Loading

simoncozens commented Dec 15, 2023

anthrotype Jan 11, 2024

anthrotype Jan 11, 2024

anthrotype commented Jan 11, 2024 •

edited

Loading

Compile variable features #635

Compile variable features #635

Conversation

simoncozens commented Jul 19, 2022 • edited Loading

madig commented Aug 2, 2022

madig Aug 2, 2022 • edited Loading

Choose a reason for hiding this comment

anthrotype Nov 28, 2023

Choose a reason for hiding this comment

simoncozens commented Aug 2, 2022

madig Aug 3, 2022

Choose a reason for hiding this comment

anthrotype Dec 5, 2023

Choose a reason for hiding this comment

madig commented Aug 3, 2022 • edited Loading

madig Aug 3, 2022

Choose a reason for hiding this comment

madig commented Aug 3, 2022 • edited Loading

simoncozens commented Aug 3, 2022

madig commented Aug 3, 2022

madig commented Aug 3, 2022

madig commented Aug 3, 2022

madig commented Aug 8, 2022

madig commented Aug 8, 2022

simoncozens commented Aug 17, 2022

simoncozens commented Aug 17, 2022

madig commented Aug 17, 2022

anthrotype commented Aug 17, 2022

madig commented Aug 23, 2022 • edited Loading

madig commented Aug 23, 2022

madig commented Aug 23, 2022

madig commented Aug 23, 2022

simoncozens commented Aug 24, 2022

madig commented Aug 24, 2022

simoncozens commented Aug 25, 2022 • edited Loading

simoncozens commented Dec 15, 2023

anthrotype Jan 11, 2024

Choose a reason for hiding this comment

anthrotype Jan 11, 2024

Choose a reason for hiding this comment

anthrotype commented Jan 11, 2024 • edited Loading

simoncozens commented Jul 19, 2022 •

edited

Loading

madig Aug 2, 2022 •

edited

Loading

madig commented Aug 3, 2022 •

edited

Loading

madig commented Aug 3, 2022 •

edited

Loading

madig commented Aug 23, 2022 •

edited

Loading

simoncozens commented Aug 25, 2022 •

edited

Loading

anthrotype commented Jan 11, 2024 •

edited

Loading